Set server_tokens to off in the nginx configuration#8
Merged
jaseemjaskp merged 1 commit intoFeb 28, 2024
Merged
Conversation
ritwik-g
approved these changes
Feb 27, 2024
jaseemjaskp
approved these changes
Feb 28, 2024
pk-zipstack
pushed a commit
that referenced
this pull request
Aug 20, 2025
…n-in-the-server-response-header-field Set server_tokens to off in the nginx configuration
muhammad-ali-e
added a commit
that referenced
this pull request
Apr 8, 2026
The "## Known Exceptions" section was previously mandatory — the contract said absence meant "the contract is not being followed" and the single line `None.` was required when the file had no exceptions. In practice this is cargo culting. The baseline for Known Exceptions is zero: most files will never have one because exceptions are rare by definition. Mandating a `None.` placeholder on every file: - Adds ~4 lines of noise per per-component file (trained readers to skip the section) - Gives the same failure mode as optional (`None.` written reflexively without the author evaluating whether exceptions exist is indistinguishable from a missing section) - Defeats the purpose of the section when a real exception is added, because readers have learned to scroll past it Change the contract to require the section only when at least one intentional, accepted exception exists. Absence of the section now means "no known exceptions today" — equivalent to `None.` but without the placeholder noise. Also tightened the Known Exceptions definition to be explicit that an entry documents an *evaluated, accepted* deviation, not "drift we haven't decided about yet." Unevaluated drift belongs in the issue tracker, not in a Known Exceptions entry. This is the lesson from the ExecutionViewSet finding on PR #1908 — a Known Exception added for unexamined drift would be fraud-by-documentation. Files changed: * design-rules/per-component-contract.md - Section structure row #8 now says the section is optional and describes the presence-only semantics. - Known Exceptions format prose updated: explicit "optional", no `None.` placeholder, entry semantics hardened to "evaluated, accepted" only. * backend/account_v2/DESIGN_RULES.md * backend/workflow_manager/DESIGN_RULES.md * unstract/connectors/DESIGN_RULES.md * unstract/connectors/src/unstract/connectors/databases/DESIGN_RULES.md * workers/shared/DESIGN_RULES.md - Dropped the empty `## Known Exceptions` / `None.` block from each prototype file. All 5 files now go straight from the last rule (or horizontal rule) to `## Checklist`. validate.sh still passes — nothing in the script depends on the Known Exceptions section being present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks
chandrasekharan-zipstack
added a commit
that referenced
this pull request
Apr 17, 2026
…e-up log Address review comments on PR #1886: - #10 (resource leak): close the generator returned by fn() before retrying in iter_with_retry — otherwise streaming providers leak an in-flight HTTP socket until GC. - #12 (behavioral regression): when we zero out SDK/wrapper retries we also lose the OpenAI SDK's native Retry-After handling on 429/503. _get_retry_delay now checks error.response.headers["retry-after"] and uses that value ahead of exponential backoff. HTTP-date form is not parsed; those fall back to backoff. - #8 (observability gap): move the "Giving up ... after N attempt(s)" log into _get_retry_delay so all four retry helpers (call_with_retry, acall_with_retry, iter_with_retry, decorator) share the same exhaustion signal. Previously only the decorator path logged it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
muhammad-ali-e
added a commit
that referenced
this pull request
Apr 17, 2026
* [FIX] Unified retry for LLM and embedding providers litellm's retry only works for SDK-based providers (OpenAI/Azure). httpx-based providers (Anthropic, Vertex, Bedrock, Mistral) and ALL embedding calls silently ignore max_retries. This adds self-managed retry with exponential backoff at the SDK layer, disabling litellm's own retry entirely for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [REFACTOR] DRY retry logic into reusable call_with_retry utilities Move retry loops out of LLM/Embedding classes into generic call_with_retry, acall_with_retry, and iter_with_retry functions in retry_utils.py. Both classes now call these directly instead of maintaining their own retry helper methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [FIX] Consolidate retry logic, expose max_retries for all adapters - Extract _get_retry_delay() shared helper to eliminate duplicated retry decision logic across call_with_retry, acall_with_retry, iter_with_retry, and retry_with_exponential_backoff - Add num_retries=0 to embedding._pop_retry_params() to fully disable litellm's internal retry for embedding calls - Expose max_retries in UI JSON schemas for embedding adapters (OpenAI, Azure, VertexAI, Ollama) and Ollama LLM — previously the field existed in Pydantic models but wasn't shown to users, silently defaulting to 0 retries - Add debug logging to LLM and Embedding retry parameter extraction - Clarify docstrings distinguishing is_retryable_litellm_error() from is_retryable_error() (different exception hierarchies) - Remove stale noqa: C901 from simplified retry_with_exponential_backoff Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [FIX] Set max_retries default to 3 for all embedding and Ollama LLM adapters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [FIX] Address greptile review: fix shadowed ConnectionError, use MRO check - Fix `requests.ConnectionError` shadowing Python's builtin `ConnectionError` in `is_retryable_litellm_error()` — rename import to `RequestsConnectionError` and use `builtins.ConnectionError` / `builtins.TimeoutError` explicitly - Use `__mro__`-based class name check instead of `type(error).__name__` to also catch subclasses of retryable error types - P1 (num_retries not zeroed) was already fixed in prior commit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [FIX] Address CodeRabbit review: add APITimeoutError, validate max_retries - Add APITimeoutError to _RETRYABLE_ERROR_NAMES for explicit OpenAI SDK timeout coverage - Add _validate_max_retries() guard to call_with_retry, acall_with_retry, iter_with_retry to fail fast on negative values instead of silently returning None Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * UN-3344 [FIX] Reduce cognitive complexity and remove useless except clause Address SonarCloud findings on PR #1886: - S3776: Flatten retry_with_exponential_backoff.wrapper by moving the success logging + return out of the try block and using `continue` in the retry path, so the except branch only handles the give-up case. - S2737: Drop the `except Exception: raise` clause — it was a no-op that added complexity without changing behavior (non-matching exceptions propagate naturally). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3344 [FIX] Extract retry loop to top-level helper to drop cognitive complexity Sonar still flagged retry_with_exponential_backoff at complexity 16 after the previous flatten. Nested def decorator / def wrapper counted against the outer function's score. Move the retry body to a module-level _invoke_with_retries helper so the decorator factory just delegates, bringing the outer function well under the 15 threshold. Behavior is unchanged — all paths (success, retry, give-up, non-retryable propagate) are preserved and covered by the existing SDK1 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3344 [FIX] Honor Retry-After, close stream gen on retry, share give-up log Address review comments on PR #1886: - #10 (resource leak): close the generator returned by fn() before retrying in iter_with_retry — otherwise streaming providers leak an in-flight HTTP socket until GC. - #12 (behavioral regression): when we zero out SDK/wrapper retries we also lose the OpenAI SDK's native Retry-After handling on 429/503. _get_retry_delay now checks error.response.headers["retry-after"] and uses that value ahead of exponential backoff. HTTP-date form is not parsed; those fall back to backoff. - #8 (observability gap): move the "Giving up ... after N attempt(s)" log into _get_retry_delay so all four retry helpers (call_with_retry, acall_with_retry, iter_with_retry, decorator) share the same exhaustion signal. Previously only the decorator path logged it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3344 [REFACTOR] Share retry-kwargs helper and add TypeVar to retry wrappers Address review comments on PR #1886: - #9 (typing): call_with_retry / acall_with_retry / iter_with_retry previously returned `object`, erasing caller type info. Add PEP 695 generics so the return type flows from the wrapped callable: acall_with_retry now takes Callable[[], Awaitable[T]] and iter_with_retry takes Callable[[], Iterable[T]] -> Generator[T, ...]. - #11 / #13 (DRY): `_pop_retry_params` in embedding.py and `_disable_litellm_retry` in llm.py were identical logic. Lift to shared `pop_litellm_retry_kwargs` helper in retry_utils.py and delete both methods. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: ali <117142933+muhammad-ali-e@users.noreply.github.com>
11 tasks
11 tasks
muhammad-ali-e
added a commit
that referenced
this pull request
May 27, 2026
…1950) * UN-3452 [FEAT] Characterise the seams: dispatch + chord call sites Sub-task A under #1.2 — characterisation suite for the seams that upcoming spine PRs will refactor. Two new test files, zero production changes. Dispatch seam (unblocks PR #8 — @shared_task -> @worker_task migration): - workers/tests/test_dispatch_sites_characterisation.py (276 lines, 11 tests) - Locks contract on the two raw current_app.send_task call sites: - shared/patterns/notification/helper.py:76 (webhook dispatch) - scheduler/tasks.py:157 (scheduled workflow async dispatch) - Tests pin: task name, positional args layout, kwargs layout, target queue, return-value semantics, error-path behaviour - Inventory canary: fails if a third raw current_app.send_task site appears anywhere in workers/ source Chord seam (unblocks PR #13 — chord -> Barrier lift): - workers/tests/test_chord_sites_characterisation.py (316 lines, 9 tests) - Locks contract on the chord pattern via: - WorkflowOrchestrationUtils.create_chord_execution (centralised helper) - WorkflowOrchestrationMixin.create_chord (mixin wrapper) - Tests pin: empty-batch short-circuit (existing defense against silent task drops at scale — Pain Point #2 in the PG Queue decision doc), callback-signature construction, return-value semantics, error propagation, mixin's app extraction + RuntimeError on missing app - Inventory canaries: fail if a third chord(...) call site OR a third `from celery import chord` import appears anywhere in workers/ source - api-deployment/tasks.py:673 inline chord covered only by inventory (direct unit-testing requires heavy mocking of the 273-line _run_workflow_api enclosing function — out of scope here, the canary still catches it for PR #13) Total: 20 tests, ~2s runtime, 0 production changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3452 [FEAT] Address Greptile review on seam characterisation tests Three P2 findings from Greptile, all fixed: 1. test_chord_import_only_in_two_files: add file-identity assertions matching the sibling call-site canary. Without these, the canary would silently pass if the two imports moved to entirely different files while count remained 2 — exactly the silent-miss scenario the Barrier migration could trigger. 2. TestSchedulerDispatchSite: add test_dispatch_returns_error_result_ when_send_task_raises. The scheduler site has a real error branch in scheduler/tasks.py:185-192 that catches send_task exceptions and returns SchedulerExecutionResult.error(...) — without a characterisation test the upcoming dispatch() migration could silently change error semantics (re-raise instead of returning an error result, or swallow silently). Mirrors the equivalent notification-site test_dispatch_returns_false_on_send_task_failure. 3. skip_dirs check anchored to top-level dir relative to workers_root in all three inventory tests. The previous `any(part in skip_dirs for part in py.parts)` check would have erroneously excluded any path with a component named `tests` (e.g. workers/shared/ tests_helpers/foo.py). 21 tests now (was 20), runtime ~3s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kirtimanmishrazipstack
added a commit
that referenced
this pull request
May 30, 2026
… hardening Addresses the in-scope items from the self-review on PR #1936: - Single-source the failure-only rule via notification_v2.helper.is_failure_run, used by api_v2 / pipeline_v2 / internal_api_views (_apply_failure_filter); the pipeline path keeps a documented last_run_status backstop. Fixes the false "parity" docstring (#1). - Emit metric= counters at the notification drop sites (backend dispatch_notifications, worker _route_notification) and a row-id sample on the dead-letter log so a delivered-never event is observable (#4). - process_notification_buffer.py honors its "never raises" contract: wrap response.json() so a non-JSON 200 returns False instead of raising (#5). - Bind the flush cap to the renderer's MAX_BATCH_SIZE so rows and rendered events stay in lock-step by construction (#7). - status db_comment now documents the PENDING -> SENDING -> DISPATCHED/DEAD_LETTER lifecycle in both the model and migration 0002 (#8). - Scrub stale IMMEDIATE / worker-callback comments from the provider docstrings (#2, #10). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Set server_tokens to off in the nginx configuration
Why
Disable emitting nginx version in the
Serverresponse header fieldHow
...
Relevant Docs
Related Issues or PRs
Dependencies Versions / Env Variables
Notes on Testing
...
Screenshots
...
Checklist
I have read and understood the Contribution Guidelines.